fix: Allow Parse Server startup without optional push adapter#10446
fix: Allow Parse Server startup without optional push adapter#10446ga262 wants to merge 5 commits intoparse-community:alphafrom
Conversation
Signed-off-by: ga262 <[email protected]>
Added a function to check for missing push adapter module and handle errors accordingly. Signed-off-by: ga262 <[email protected]>
Removed '@parse/push-adapter' from dependencies and added it to optionalDependencies. Signed-off-by: ga262 <[email protected]>
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMoved ChangesPush adapter optionalization and handling
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.2.525)package.json2026-05-02 10:19:06,280 [MainThread ] [ERROR] Template file not found: package.json ... [truncated 2548 characters] ... k__) Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Controllers/index.js (1)
172-201:⚠️ Potential issue | 🟠 MajorOnly load the default push adapter when it is actually needed.
Line 191 unconditionally imports the optional default adapter even when
pushis unset or a custompush.adapteris provided. This can cause startup to fail unnecessarily when the adapter is not used. Additionally, the error detection at line 176 usingmessage.includes('@parse/push-adapter')can misclassify transitive dependency failures from within@parse/push-adapteras a missing package if the error message path happens to contain that string.Move the import into the conditional block that only executes when actually needed:
Suggested fix
function isPushAdapterModuleMissing(error: any): boolean { const message = `${error?.message || error || ''}`; const hasMissingCode = error?.code === 'ERR_MODULE_NOT_FOUND' || error?.code === 'MODULE_NOT_FOUND'; - return hasMissingCode && message.includes('@parse/push-adapter'); + return ( + hasMissingCode && + /Cannot find (?:package|module) ['"]@parse\/push-adapter['"]/.test(message) + ); } // Pass the push options too as it works with the default let ParsePushAdapter; - try { - ParsePushAdapter = await loadModule('@parse/push-adapter'); - } catch (error) { - if (!isPushAdapterModuleMissing(error)) { - throw error; - } - if (push && !pushOptions.adapter) { + if (push && !pushOptions.adapter) { + try { + ParsePushAdapter = await loadModule('@parse/push-adapter'); + } catch (error) { + if (!isPushAdapterModuleMissing(error)) { + throw error; + } throw new Error( 'Push is configured but the optional dependency "@parse/push-adapter" is not installed. Install "@parse/push-adapter" or configure "push.adapter".' ); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Controllers/index.js` around lines 172 - 201, The code currently calls loadModule('@parse/push-adapter') unconditionally and uses isPushAdapterModuleMissing(error) which naively checks message.includes('@parse/push-adapter'), causing unnecessary startup failures and false positives; update getPushController so ParsePushAdapter is only loaded when push is truthy and pushOptions.adapter is not provided (i.e. move the await loadModule('@parse/push-adapter') call inside the conditional that checks if push && !pushOptions.adapter), and harden isPushAdapterModuleMissing(error) to detect a missing package more precisely by checking error.code for 'ERR_MODULE_NOT_FOUND' or 'MODULE_NOT_FOUND' and matching the error message against a stricter pattern that looks for "Cannot find module" (or the module name quoted) specifically for '@parse/push-adapter' rather than a simple substring match.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/Controllers/index.js`:
- Around line 172-201: The code currently calls
loadModule('@parse/push-adapter') unconditionally and uses
isPushAdapterModuleMissing(error) which naively checks
message.includes('@parse/push-adapter'), causing unnecessary startup failures
and false positives; update getPushController so ParsePushAdapter is only loaded
when push is truthy and pushOptions.adapter is not provided (i.e. move the await
loadModule('@parse/push-adapter') call inside the conditional that checks if
push && !pushOptions.adapter), and harden isPushAdapterModuleMissing(error) to
detect a missing package more precisely by checking error.code for
'ERR_MODULE_NOT_FOUND' or 'MODULE_NOT_FOUND' and matching the error message
against a stricter pattern that looks for "Cannot find module" (or the module
name quoted) specifically for '@parse/push-adapter' rather than a simple
substring match.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 950bd482-0f92-4d9f-b1b7-633395bf13d3
📒 Files selected for processing (3)
package.jsonspec/index.spec.jssrc/Controllers/index.js
Issue
Parse Server currently declares
@parse/push-adapteras an optional dependency, but startup still attempts to load it unconditionally in the push controller path. This creates a mismatch between package metadata and runtime behavior.Current behavior
@parse/push-adapteris not installed:push.adapter, startup fails with a generic module resolution errorExpected behavior
pushis not configured, Parse Server should start even if@parse/push-adapteris absentpushis configured but no custom adapter is provided, Parse Server should fail with a clear actionable error indicating that@parse/push-adaptermust be installed (orpush.adaptermust be provided)Why this matters
optionalDependenciesintent--omit=optional)Summary
@parse/push-adapterwhen loading the default push adapterpushis not configured and the optional adapter is not installedpushis configured but neither@parse/push-adapternor a custompush.adapteris availablePushWorkerwhen push support is enabledTest plan
npm run buildnpm run testonly spec/index.spec.jsnpm run testonly spec/AdapterLoader.spec.jspushisundefinedand@parse/push-adapteris missingpushis configured and@parse/push-adapteris missingSummary by CodeRabbit
New Features
Improvements
Tests